Skip to content

[WIP] IX Reorganization and IX/GAC Parmest Demo #1636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
kurbansitterley wants to merge 142 commits into
base: main
Choose a base branch
from
Draft

[WIP] IX Reorganization and IX/GAC Parmest Demo #1636

kurbansitterley wants to merge 142 commits into from

Conversation

@kurbansitterley
Copy link
Contributor

@kurbansitterley kurbansitterley commented Sep 1, 2025 •
edited
Loading

Fixes/Resolves:

This PR is a WIP and will probably need to be broken up into a few smaller PRs. It reorganizes the existing IX model into a base class and the models inherit from that base class. It also adds a parameter estimation demo for IX and GAC breakthrough curves.

Relies on #1548

Summary/Motivation:

Have a PR to work off of for the development of the demo.

Changes proposed in this PR:

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@kurbansitterley
initial refactor
35da23d
@kurbansitterley
initial separate for Clark model
5f02b6a
@kurbansitterley
initial separate for perry/lang
c302f93
@kurbansitterley
test harness ix
1ca52ac
@kurbansitterley
initial commit base
4350daa
@kurbansitterley
initial ix demin model
611a76e
@kurbansitterley
small rework of base model checkpoint
23ed276
@kurbansitterley
__init__s
3d570ed
@kurbansitterley
move some commented constraints to perry's
59015a1
@kurbansitterley
initial costing changes for ix reorg
cd90a57
@kurbansitterley
model checkpoints
e678380
@kurbansitterley
Merge branch 'main' of https://github.com/kurbansitterley/watertap in…
2d6e040 
…to ix_reorg
@kurbansitterley
demin checkpoint
a78317b
@kurbansitterley
base checkpoint
7b9af00
@kurbansitterley
use equivalent flow
ad8ecfb
@kurbansitterley
demin checkpoint
adf693a
@kurbansitterley
base working with demin
4815354
@kurbansitterley
demin with charge ratio
c8b819c
@kurbansitterley
add IX demineralize costing option
27b6da5
@kurbansitterley
add scaling to demin
da5c9f6
@kurbansitterley
create ix testing directory
38dcdca
@kurbansitterley
initial ix demin test
6384583
@kurbansitterley
clean up base
d7fe6c7
@kurbansitterley
clean up demin
7646416
@kurbansitterley
remove local attribute to resin cost
b7a40ca
@kurbansitterley
add back old model for comparison
983e63b
@kurbansitterley
add clark model
6560732
@kurbansitterley
working clark test
ea1a71b
@kurbansitterley
rename perry to constant_pattern
104071d
@kurbansitterley
add two refs to const pattern
09a939b
@adam-a-a
Copy link
Contributor

adam-a-a commented Sep 1, 2025

I should also add that I think the demo (and all the other work that you put into this PR) is incredible!

@kurbansitterley
Copy link
Contributor Author

kurbansitterley commented Sep 1, 2025

My high-level takeaway on the notebook is that it is very detailed---maybe too detailed for a code demo to stakeholders

I think I agree with this. I initially had it set up to just import the necessary code/modules, but thought that might detract from the demo a bit as the actual code presented was more sparse. There is certainly a balance to strike between showing everything with detail and keeping it all behind an import curtain.

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Sep 11, 2025
@kurbansitterley kurbansitterley self-assigned this Sep 11, 2025
@kurbansitterley
index some ss constraints and vars
07465a7
@kurbansitterley
refactor for indexed ss constraints; update tests
5d38ad3
@kurbansitterley
refactor add_ss_approximation: improve c_traps calculation with smoot…
2b00318 
…h_max and reactive_component_set for index
@kurbansitterley
merge with multicomponent
e69adc7
@kurbansitterley
add multi component test initial
e8a7541
@kurbansitterley
checkpoint for constant pattern model
e73cc3f
@kurbansitterley
Merge branch 'main' of https://github.com/kurbansitterley/watertap in…
c831952 
…to ix_reorg
@kurbansitterley
add demineralize as ix_type option
ca7e10d
@kurbansitterley
add CONFIG for num_traps; clean up
2ade1e5
@kurbansitterley
remove multicomponent functionality for now
c89393f
@kurbansitterley
skip multicomponent tests for now
615044c
@kurbansitterley
comment existing IX 0d test
4fcc323
@kurbansitterley
remove multi component model and test file
7cf8e4d
@kurbansitterley
remove ix parmest model, test, and data
b05065c
@kurbansitterley
remove multicomponent and param est models from init
0b8c660
@kurbansitterley
fix initialization for cphsdm
2a5f34c
@kurbansitterley
passing tests for cphsdm
e096a14
@kurbansitterley
remove ix/gac parmest demo
8b9cc92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adam-a-a adam-a-a adam-a-a left review comments

@MarcusHolly MarcusHolly Awaiting requested review from MarcusHolly MarcusHolly will be requested when the pull request is marked ready for review MarcusHolly is a code owner

@bknueven bknueven Awaiting requested review from bknueven bknueven will be requested when the pull request is marked ready for review bknueven is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@kurbansitterley kurbansitterley

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kurbansitterley @adam-a-a @ksbeattie @hunterbarber